fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

daniellacosse · 2024-02-15T23:31:35Z

I was getting multiple errors attempting to build the latest release for ios:

It seems we require a greater deployment target. The xcode CLI won't build for 11, saying the supported range is 12 - 17.
Our security group entitlements for ios had macos specific groups in them, which I assume were being ignored before.

codecov · 2024-02-15T23:34:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 32%. Comparing base (81abbb9) to head (c8f74ec).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #1840    +/-   ##
=======================================
- Coverage      40%     32%    -8%     
=======================================
  Files          39      45     +6     
  Lines        1805    2610   +805     
  Branches      337     337            
=======================================
+ Hits          738     859   +121     
- Misses       1067    1751   +684

Flag	Coverage Δ
apple	`15% <ø> (?)`
ios	`15% <ø> (?)`
maccatalyst	`15% <ø> (?)`
macos	`15% <ø> (?)`
unittests	`32% <ø> (-8%)`	⬇️
www	`40% <ø> (-1%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fortuna · 2024-02-16T00:09:08Z

src/cordova/apple/xcode/ios/Outline/Outline.entitlements

@@ -11,7 +11,6 @@
 	<key>com.apple.security.application-groups</key>
 	<array>
 		<string>group.org.outline.ios.client</string>
-		<string>$(TeamIdentifierPrefix)org.outline.macos.client</string>


@sbruens will this break mac Catalyst?

I don't understand why having this would break the build, since an app can be in multiple application groups. Perhaps we need to configure the application group to include the ios app.

when it fails it says that the provision (org.outline.ios.client) doesn't match what's in this group (e.g. the macos group). perhaps we can use the interpolation to change this at compile time?

This will most likely break macOS, because they share this file.
Please wait for @sbruens to chime in.
Other than that, Makefile and pbxproj look good.

It will break catalyst because this app group is used in several places to share data between the network extension, launcher and host app, like the logs directory for Sentry and the tunnel store (see https://github.com/search?q=repo%3AJigsaw-Code%2Foutline-client%20%22QT8Z3Q9V3A.org.outline.macos.client%22&type=code).

Those values need to match the app groups the app has access to. I think we may be able to move catalyst over to use the ios app group but it would mean that the catalyst app would no longer use the existing mac paths for logs and tunnel store preferences, but that might be ok?

Btw I added those entitlements very recently: #1811

Perhaps the iOS app provisioning profile needs to be updated online to add those groups?

I think the problem is that macOS groups are special and not managed in the Developer portal. So you can't even create an app group that doesn't start with group..

That app groups shouldn't be set on the iOS app anyway. But the XCode UI is weird where it automatically adds macOS app groups to the iOS section (but not the other way around?). Is there a way to set them in the release script?

I guess the best course of action would be to update those hard-coded values and move catalyst over to the iOS app group.

Will the user lose their server list? That would be a very bad experience.
Perhaps we need separate projects? Can we have a per-platform plist?

I don't think so. They are not stored under the app group.

@daniellacosse you should be able to check that by running the catalyst app after you change these values and checking that your servers show up.

Yep, still seeing my servers:

src/cordova/apple/xcode/ios/Outline.xcodeproj/project.pbxproj

jyyi1

Not sure whether it's related (maybe not), but @sbruens and I discovered that the latest Gomobile behaves weirdly until a go get xxx/gomobile is run targeting the root folder.

Makefile

fortuna · 2024-02-23T02:14:58Z

You shouldn’t use go get to install binaries, because who knows what version you’ll use. The right way is tools.go, pinned version on go.sum (use go mod tidy), then go build.

…

On Thu, Feb 22, 2024 at 8:11 PM J. Yi ***@***.***> wrote: ***@***.**** commented on this pull request. Not sure whether it's related (maybe not), but @sbruens <https://github.com/sbruens> and I discovered that the latest Gomobile behaves weirdly until a go get xxx/gomobile is run targeting the root folder. ------------------------------ In Makefile <#1840 (comment)> : > @@ -23,7 +23,7 @@ $(BUILDDIR)/android/tun2socks.aar: $(GOMOBILE) $(BUILDDIR)/ios/Tun2socks.xcframework: $(GOMOBILE) # -iosversion should match what outline-client supports. - $(GOBIND) -iosversion=11.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks + $(GOBIND) -iosversion=12.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks Will this drop the support of iOS 11 ? If so, do we need to announce it somewhere, maybe in Reddit? — Reply to this email directly, view it on GitHub <#1840 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3XHL7SH4G7YTS6ODDGZTYU7UFBAVCNFSM6AAAAABDLEALFOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJXGI2DSNZQHA> . You are receiving this because your review was requested.Message ID: ***@***.***>

daniellacosse · 2024-02-23T17:06:47Z

I thought we had that and it wasn't working for some reason? I forget On Thu, Feb 22, 2024 at 9:15 PM Vinicius Fortuna ***@***.***> wrote:

…

You shouldn’t use go get to install binaries, because who knows what version you’ll use. The right way is tools.go, pinned version on go.sum (use go mod tidy), then go build. On Thu, Feb 22, 2024 at 8:11 PM J. Yi ***@***.***> wrote: > ***@***.**** commented on this pull request. > > Not sure whether it's related (maybe not), but @sbruens > <https://github.com/sbruens> and I discovered that the latest Gomobile > behaves weirdly until a go get xxx/gomobile is run targeting the root > folder. > ------------------------------ > > In Makefile > < #1840 (comment)> > : > > > @@ -23,7 +23,7 @@ $(BUILDDIR)/android/tun2socks.aar: $(GOMOBILE) > > $(BUILDDIR)/ios/Tun2socks.xcframework: $(GOMOBILE) > # -iosversion should match what outline-client supports. > - $(GOBIND) -iosversion=11.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks > + $(GOBIND) -iosversion=12.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks > > Will this drop the support of iOS 11 ? If so, do we need to announce it > somewhere, maybe in Reddit? > > — > Reply to this email directly, view it on GitHub > < #1840 (review)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAA3XHL7SH4G7YTS6ODDGZTYU7UFBAVCNFSM6AAAAABDLEALFOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJXGI2DSNZQHA> > . > You are receiving this because your review was requested.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#1840 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4V5VHVSWWSVGBAWWUJQNDYU73S7AVCNFSM6AAAAABDLEALFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRQGY2DGNBSGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jyyi1

LGTM. Please also let the QA team test the following behavior on macOS once the PR is merged:

Install an old version
Add some servers (ss:// and ssconf://)
Upgrade to the new version
Make sure all servers and configurations are still there

daniellacosse · 2024-02-27T21:36:06Z

LGTM. Please also let the QA team test the following behavior on macOS once the PR is merged:

Install an old version

Add some servers (ss:// and ssconf://)

Upgrade to the new version

Make sure all servers and configurations are still there

I'm gonna merge this, considering it's semi-blocking release

fix(cordova/apple/ios): fix failing build for the cordova ios target

6678509

github-actions bot added the size/XS label Feb 15, 2024

daniellacosse changed the title ~~fix(cordova/apple/ios): fix failing build for the cordova ios target~~ fix(cordova/apple/ios): fix failing release build for the cordova ios target Feb 15, 2024

update entitilements

41f6a22

daniellacosse marked this pull request as ready for review February 15, 2024 23:39

daniellacosse requested a review from a team as a code owner February 15, 2024 23:39

fortuna reviewed Feb 16, 2024

View reviewed changes

fortuna requested a review from sbruens February 16, 2024 00:22

update makefile

2807d7d

daniellacosse requested a review from fortuna February 22, 2024 15:09

jyyi1 reviewed Feb 23, 2024

View reviewed changes

Makefile Show resolved Hide resolved

daniellacosse requested a review from jyyi1 February 24, 2024 12:03

jyyi1 approved these changes Feb 26, 2024

View reviewed changes

Merge branch 'master' into daniellacosse/ios_fix

c8f74ec

daniellacosse changed the title ~~fix(cordova/apple/ios): fix failing release build for the cordova ios target~~ fix(client/cordova/apple/ios): fix failing release build for the cordova ios target Feb 27, 2024

daniellacosse merged commit f3e10d6 into master Feb 27, 2024
25 of 26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

daniellacosse commented Feb 15, 2024 •

edited

Loading

codecov bot commented Feb 15, 2024 •

edited

Loading

fortuna Feb 16, 2024

daniellacosse Feb 16, 2024 •

edited

Loading

fortuna Feb 16, 2024

sbruens Feb 16, 2024 •

edited

Loading

sbruens Feb 16, 2024

fortuna Feb 16, 2024

sbruens Feb 20, 2024 •

edited

Loading

fortuna Feb 20, 2024

sbruens Feb 20, 2024

daniellacosse Feb 22, 2024

jyyi1 left a comment

fortuna commented Feb 23, 2024 via email

daniellacosse commented Feb 23, 2024 via email

jyyi1 left a comment

daniellacosse commented Feb 27, 2024

fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

Conversation

daniellacosse commented Feb 15, 2024 • edited Loading

codecov bot commented Feb 15, 2024 • edited Loading

Codecov Report

fortuna Feb 16, 2024

Choose a reason for hiding this comment

daniellacosse Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

fortuna Feb 16, 2024

Choose a reason for hiding this comment

sbruens Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

sbruens Feb 16, 2024

Choose a reason for hiding this comment

fortuna Feb 16, 2024

Choose a reason for hiding this comment

sbruens Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

fortuna Feb 20, 2024

Choose a reason for hiding this comment

sbruens Feb 20, 2024

Choose a reason for hiding this comment

daniellacosse Feb 22, 2024

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

fortuna commented Feb 23, 2024 via email

daniellacosse commented Feb 23, 2024 via email

jyyi1 left a comment

Choose a reason for hiding this comment

daniellacosse commented Feb 27, 2024

daniellacosse commented Feb 15, 2024 •

edited

Loading

codecov bot commented Feb 15, 2024 •

edited

Loading

daniellacosse Feb 16, 2024 •

edited

Loading

sbruens Feb 16, 2024 •

edited

Loading

sbruens Feb 20, 2024 •

edited

Loading